Add new --follow mode for tower apps logs with resilient streaming and tests
#171
Conversation
- deduplicate log lines across reconnects / might be an issue - add unit test for deduplication and out of order logs
|
Nice thanks @burakdede, we should make the default branch in this repo I'll review the content now.! |
There was a problem hiding this comment.
Besides the failure re: linting, this is generally really good! I'd really love to merge this with the following logic in the do_run function in crates/tower-cmd/src/run.rs -- but I can manage that on our side! All the stuff around retries, etc., is really good and missing from the other implementation.
I'll leave this open for a bit just in case you decide this is something you want to tackle, but otherwise I'll rebase this against develop and land this over the weekend!
| if is_run_finished(&run) { | ||
| if let Ok(resp) = api::describe_run_logs(&config, &name, seq).await { | ||
| for line in resp.log_lines { | ||
| emit_log_if_new(&line, &mut last_line_num); | ||
| } | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
This is good, we really should unify these endpoints at some point to make this easier 🙃
| let (cancel_tx, cancel_rx) = oneshot::channel(); | ||
| cancel_monitor = Some(cancel_tx); | ||
| let run_complete = monitor_run_completion(&config, &name, seq, cancel_rx); | ||
| match api::stream_run_logs(&config, &name, seq).await { |
There was a problem hiding this comment.
This should be fine functionally, but comparing it to our implementation for starting a remote run and following the logs, we wait for the run to start first...that may not be necessary, we should be able to just start listening? I'll double check that.
2 participants
Addresses improvement required #164
--followtotower apps logswith SSE streaming, retry/backoff, Ctrl+C handling, dedupe across reconnects, and fatal‑error exits.Tests should cover most of the critical aspects of the implementation but I have also simulated some tests via local
towerbuild and deploying app for;Broken pipeand CLI panic #162 still fails when piping to head (known issue).Drain Logs & Clean Exit
Intermittent Failure Results & Slightly-Graceful Exit
Kill Log Stream & Clean Exit